Conversation
Phase 1 of the native RDP client architecture. The bridge accepts an inbound RDP connection, opens an outbound connection to a Windows target, injects credentials via CredSSP/NLA on the target side, then byte-forwards raw TLS traffic in both directions. The acceptor and connector state machines are driven only up through CredSSP and then stopped. Client and target negotiate MCS, channels, capabilities, and share state directly through the passthrough pipe, which avoids the capability-flag drift and channel-ID drift that naive dual-handshake forwarding produces and that strict clients (Windows App, mstsc) reject. Validated end-to-end against a real Windows Server 2022 target with both sdl-freerdp (permissive) and Microsoft Windows App (strict). Phase 1 scope: standalone test binary only. No FFI, no event tap, no session recording. Crate produces a staticlib for Phase 2 CGo linking.
Phase 2 of the native RDP client architecture. Exposes the Rust bridge as a handle-based C ABI and wraps it in an idiomatic Go package, making the bridge callable from gateway and CLI code. Rust side: - Handle-based FFI in src/ffi.rs: start/wait/cancel/free. Each session runs on a dedicated OS thread with its own current-thread tokio runtime, isolating failures between sessions. - Public C header at native/include/rdp_bridge.h. - run_mitm now accepts a CancellationToken so sessions can be aborted from any thread; tokio::select! in the bridge races the MITM future against token.cancelled() and drops the future on cancel, closing TCP streams automatically. Go side (packages/pam/handlers/rdp/): - bridge.go: always-compiled types and error sentinels. - bridge_cgo.go: real CGo wrapper, gated on `rdp && (linux || darwin)`. StartWithConn dups the caller's fd via syscall.Conn/Control so Go and Rust own independent references to the socket. - bridge_stub.go: ErrRdpUnavailable stubs for builds without -tags rdp or on platforms without the static lib yet (windows/freebsd/netbsd land in Phase 6). - cmd/bridge-test: Go harness that exercises the full Rust -> C ABI -> CGo -> Go path. Validated end-to-end against the existing Windows Server target with Microsoft Windows App. No product code calls this yet; Phase 3 wires the gateway handler.
Phase 3 of the native RDP client architecture. The gateway's existing PAM dispatcher now routes Windows/RDP sessions to the Rust bridge. Changes: - Add ResourceTypeRDP = "windows" constant (matches backend enum). - Extend GetSupportedResourceTypes to advertise RDP. - Add a session.ResourceTypeRDP case to HandlePAMProxy's switch that builds an rdp.RDPProxyConfig from the session credentials and calls proxy.HandleConnection. Target port is range-validated before downcast to uint16. New handler API in packages/pam/handlers/rdp: - RDPProxy + RDPProxyConfig + NewRDPProxy in proxy.go (shared across all builds so the dispatcher can always reference the type). - HandleConnection uses StartWithReadWriter (new) to set up a local loopback TCP pair: the gateway's *tls.Conn gets pumped through two io.Copy goroutines to/from the loopback peer, while the Rust bridge owns the other end as a raw kernel fd. This is required because the gateway's conn is TLS-wrapped over an SSH channel and does not expose syscall.Conn. - Bridge gains an optional cleanup hook called during Close, which StartWithReadWriter uses to tear down the loopback goroutines. - Supervisor in HandleConnection translates ctx.Done into bridge.Cancel so admin terminate and session expiry propagate cleanly. - Stub file gains a HandleConnection that closes the conn and returns ErrRdpUnavailable, keeping the dispatcher compilable on builds without -tags rdp. No gateway transport changes needed: RDP reuses the existing `infisical-pam-proxy` ALPN, and routing happens via the ResourceType field the backend puts in the client certificate extension. Full end-to-end validation deferred to Phase 4 when the CLI side exists.
Phase 4 of the native RDP client architecture. Wires up `infisical pam rdp access --resource <name> --account <name>`. The command: - Authenticates the user and creates a PAM session via the existing CallPAMAccessWithMFA / HandleApprovalWorkflow plumbing. - Validates that the connected gateway advertises RDP support in its capabilities response. - Binds a loopback TCP listener on 127.0.0.1 (port 0 by default, or user-supplied via --port). - Writes a minimal .rdp file to the OS temp dir pointing at the loopback, prefilled with the fixed acceptor username. - Auto-launches the system RDP client (`open` on macOS, `cmd /c start` on Windows, `xdg-open` on Linux) unless --no-launch is passed. - Accepts one RDP client connection at a time and forwards bytes to the gateway over the existing mTLS+SSH relay with the standard infisical-pam-proxy ALPN. The gateway's Phase 3 dispatcher routes to the Windows handler, which spawns the Rust bridge. - Translates Ctrl-C to a graceful session teardown: notify the gateway via the cancellation ALPN, drain active connections, exit. New files: - packages/pam/local/rdp-proxy.go: RDPProxyServer mirrors DatabaseProxyServer (accept loop + io.Copy forwarding), plus writeRDPFile / launchRDPClient helpers. Modified: - packages/cmd/pam.go: adds the pam rdp access cobra command. - packages/pam/handlers/rdp/bridge.go: exports AcceptorUsername and AcceptorPassword consts kept in sync with the Rust crate, so the CLI can print them and embed them in the .rdp file. End-to-end validation needs Phase 5 (backend support for the Windows resource type); the CLI is otherwise feature-complete and builds cleanly both with -tags rdp (real bridge) and without (stub returning ErrRdpUnavailable).
- Write .rdp files to ~/.infisical/rdp/ instead of the OS temp dir so they live next to the CLI's other per-user state (login config, update-check cache). Falls back to os.TempDir() if the home directory can't be resolved. Directory is created on first use with 0700 permissions. - Copy the fixed acceptor password to the system clipboard when the proxy starts, so the user can paste it into the RDP client prompt instead of typing `infisical` each time. No .rdp file format has a portable way to embed a plaintext password (mstsc's DPAPI field is Windows-local; Mac / freerdp clients ignore any password field at all), so the clipboard is the cleanest universal answer. Uses pbcopy / clip / xclip-or-xsel per-OS; failure is non-fatal.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 32113716 | Triggered | Generic Password | 54e921b | dev/pam/resources/ssh-server/entrypoint.sh | View secret |
| 32113716 | Triggered | Generic Password | 54e921b | dev/pam/resources/mssql/entrypoint.sh | View secret |
| 32113717 | Triggered | Generic Password | 54e921b | dev/pam/resources/mongodb/init/seed.js | View secret |
| 32113718 | Triggered | Username Password | 54e921b | dev/pam/setup.sh | View secret |
| 32113718 | Triggered | Username Password | 54e921b | dev/pam/docker-compose.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Pins the Rust channel in rust-toolchain.toml so local dev and the three Phase 6 cross-compile CI jobs (coming next) all agree on the compiler version. Adds run-cli-rdp-smoke.yml: a per-PR job that builds the Rust static library for linux/amd64, runs fmt/clippy, builds the CLI with -tags rdp, and smoke-tests the binary (--version, `pam rdp access --help`). Fast guard against regressions that would otherwise only surface when cutting a release. Path-filtered to packages/pam/** + packages/cmd/pam.go so it doesn't fire on unrelated frontend or backend CLI work.
Adds a dry_run boolean input to workflow_dispatch. When triggered with dry_run=true (the default for manual runs), goreleaser runs in --snapshot --skip=publish mode on both the linux and windows jobs, the S3 upload / Cloudfront invalidation / npm publish steps are skipped, and the built binaries are uploaded as workflow artifacts (goreleaser-dist-linux + goreleaser-dist-windows). The normal tag-push flow (github.event_name == 'push') is unchanged: full release, all publish steps run. Purpose: lets us exercise the full cross-compile matrix end-to-end during Phase 6 work without cutting a real release. We trigger from the Actions tab, download the dist artifact, spot-check the binaries locally.
Adds build-rdp-bridge.yml with three jobs:
- rust-cross (ubuntu-latest-8-cores): 8 targets via `cross` — linux/{386,
amd64, arm64, armv6, armv7}, freebsd/amd64, netbsd/amd64, windows/amd64
- rust-darwin (macos-latest): darwin/amd64 + darwin/arm64 natively
- rust-winarm (windows-11-arm): windows/arm64 natively via MSVC
Each matrix leg uploads its static archive as a workflow artifact
named rdp-bridge-<rust-triple> that the release workflow will
download and link per-target via CGO_LDFLAGS (wiring comes next).
The workflow is workflow_call + workflow_dispatch: invoked as a
prerequisite to goreleaser on real releases, and triggerable manually
for iterating on the cross-compile matrix without a full release run.
Also broadens run-cli-rdp-smoke.yml trigger to fire on every PR sync
(dropping the packages/pam/** paths filter) so it stays useful during
Phase 6 work where most commits touch .github/ rather than source.
Windows support for the RDP bridge: bridge_cgo_windows.go uses DuplicateHandle for in-process SOCKET duplication (in-process, not WSADuplicateSocketW which is for cross-process sharing) and the same loopback shim for non-socket-backed streams. Extracted Wait/Cancel/Close into bridge_cgo_shared.go since the C calls are platform-independent. The stub build tag is narrowed to exclude windows now that windows has a real implementation.
Scope is 'client tier' only: linux amd64/arm64, darwin amd64/arm64, and windows amd64 via MinGW. Other targets (linux/386, 32-bit arm, BSDs, windows-arm/arm64) keep CGO=0 and ship the stub, which returns ErrRdpUnavailable at runtime. Changes: * build-rdp-bridge.yml pruned from 11 targets to 5 (windows/amd64 is now x86_64-pc-windows-gnu so cgo cross-compile from ubuntu can link against it with MinGW). The rust-winarm MSVC job is removed. * .goreleaser.yaml splits the old all-other-builds group. Five new per-arch builds (darwin-amd64-rdp, darwin-arm64-rdp, linux-amd64-rdp, linux-arm64-rdp, windows-amd64-rdp) compile CGO=1 -tags rdp against the corresponding cargo target/<triple>/release/libinfisical_rdp_bridge.a via CGO_LDFLAGS. The remaining all-other-builds group covers the stub-tier platforms with CGO=0. * release_build_infisical_cli.yml adds build-rdp-bridge as a needs prereq for the goreleaser job, installs gcc-aarch64-linux-gnu and gcc-mingw-w64-x86_64 on the ubuntu runner, downloads the five artifacts, and stages each one at its expected cargo output path.
Mirrors the pattern the other PAM proxies adopted on main (database / ssh / kubernetes / redis): resolveReason reads --reason or prompts the user when interactive, the reason is threaded into the PAMAccessParams, and CallPAMAccessWithMFA is called with interactive = true so the backend's reason-required policy triggers a prompt instead of failing silently.
Two issues from the first dry-run of the RDP release pipeline: 1. gcc-mingw-w64-x86_64 doesn't exist as an Ubuntu apt package name (underscore vs hyphen). The correct name is gcc-mingw-w64-x86-64. 2. The windows-2022 runner doesn't always have a running docker daemon, so goreleaser fails trying to build the Windows Server container images in dry-run mode. Skip docker only for dry-run; real releases keep building them as before.
Previous commit claimed the windows docker build was pre-existing broken, but real releases do in fact produce those images. Drop the --skip=docker workaround and let the next dry-run show whether docker actually recurs or the first failure was transient.
The previous osxcross-target clone only provided the macOS SDK headers, not a real linker. That worked when no cgo code was actually being linked, but with -tags rdp pulling in bridge_cgo.go the final link step falls through to /usr/bin/ld (GNU) which can't handle ld64-specific flags like -headerpad and -framework, producing 'unrecognised emulation mode: llvm'. zig cc ships a full clang frontend plus ld.lld (with ld64 compat) plus a bundled macOS SDK snapshot, so it can produce valid Mach-O from linux. Scoped to the two darwin rdp builds only. Linux and windows builds keep their existing native / mingw toolchains.
Every CLI target except openbsd now ships with RDP support. Openbsd stays on the stub tier since we don't have a rust/cargo cross for it and it's a negligible share of PAM users. Changes: * build-rdp-bridge.yml: add back i686-unknown-linux-gnu, arm-unknown-linux-gnueabi, armv7-unknown-linux-gnueabihf, x86_64-unknown-freebsd, x86_64-unknown-netbsd to the cross job matrix. Add a new rust-winarm job that cross-compiles aarch64-pc-windows-gnullvm via native cargo + rust-lld (the cross tool doesn't cover this target). * .goreleaser.yaml: six new per-arch rdp builds (linux-386, linux-armv6, linux-armv7, freebsd-amd64, netbsd-amd64, windows-arm64). BSDs and windows-arm64 use zig cc as the linker (same toolchain already used for darwin) because apt doesn't ship cgo-capable cross compilers for them. all-other-builds narrows to openbsd only; nfpms .deb/.rpm/.apk now covers all five linux arches. * release_build_infisical_cli.yml: install gcc-i686-linux-gnu, gcc-arm-linux-gnueabi, gcc-arm-linux-gnueabihf. Expand the bridge staging loop to all 11 triples.
The session/ directory holds encrypted per-user session state written by `infisical pam rdp access` during local testing. It has no place in the repo; adding it to .gitignore so it doesn't happen again.
The linux/arm64 (and by extension all linux cross-compile) goreleaser builds were failing with 'cannot find -lz' because Ubuntu's cross-gcc packages don't ship per-arch zlib. Pinning libz-sys with the static feature makes it compile zlib from source (via cc) and link it statically into libinfisical_rdp_bridge.a, so the consuming CGO link step doesn't need a system -lz for any target arch. libz-sys is a transitive dep of flate2, which ironrdp's smartcard / CredSSP paths pull in through winscard -> sspi. We don't use those code paths at runtime, but the symbols still get compiled into the static archive. Also drop -lz from the Linux and Darwin LDFLAGS directives in bridge_cgo.go since the symbols are now baked into the .a.
aarch64-pc-windows-gnullvm cross-compile requires aarch64-w64-mingw32-clang from llvm-mingw (not in Ubuntu's apt repos) because libz-sys's cc crate needs it to build the bundled zlib. Between the extra toolchain surface and the negligible PAM user share on windows/arm64, it's cleaner to keep this target on the CGO=0 stub tier. Moves windows/arm64 back into all-other-builds alongside openbsd. 10 RDP-enabled targets total, down from 11.
The bridge crate has two targets: the libinfisical_rdp_bridge.a staticlib (what we ship) and an rdp-bridge-test dev binary (local convenience for exercising the MITM flow against a real RDP server). The bin target pulls in extra runtime deps that cross's default sysroots don't ship. Hit it first on NetBSD, which needs libexecinfo for backtrace support. Adding --lib skips the bin build entirely, makes the job faster across the board, and avoids the whole class of 'target sysroot missing dev-binary dep' issues.
Most of the comment blocks I'd added were restating the code rather than explaining non-obvious WHY. Kept the ones that actually matter (Windows App protocol-echo requirement, DuplicateHandle vs WSADuplicateSocketW, the UnexpectedEof/close_notify quirk, why libz-sys static is needed for cross-compile) and dropped the rest.
zig cc doesn't bundle FreeBSD/NetBSD libc headers, so cgo's <stdlib.h> include fails with 'stdlib.h file not found' during the Go link step. No standard Ubuntu apt package ships a cgo-capable BSD sysroot either. Moving both back to the CGO=0 stub alongside openbsd and windows/arm64. Final RDP matrix: 8 targets (5 linux arches + windows/amd64 + darwin amd64/arm64).
Verified locally that zig 0.16.0 resolves -lresolv correctly when cross-compiling cgo net package to darwin (the issue that broke darwin_amd64_v1 on the last dry-run). 0.13.0 didn't ship a libresolv SDK stub.
.rdp file cleanup: the generated file sticks in ~/.infisical/rdp/ indefinitely. It's dead weight once the CLI exits: the proxy port is closed, so double-clicking the file just fails. Added removal inside gracefulShutdown, placed before p.cancel() so main doesn't race ahead and exit the process before the removal completes. Ignores os.IsNotExist (if the user already deleted it manually) and any other error (best-effort, don't fail shutdown on filesystem hiccups). Zig pin: 0.16.0 isn't a stable release on ziglang.org (brew was shipping a dev build), so mlugg/setup-zig@v1 couldn't locate it. Verified locally that 0.14.0 correctly resolves -lresolv when cross-compiling cgo net package to darwin, which was the underlying libresolv issue we bumped zig to fix.
My earlier claim that 0.14/0.16 'fixed' libresolv was wrong: I was fooled by Go's internal linker on a macOS host, which skipped the external zig cc invocation entirely. Direct invocation of 'zig cc -target x86_64-macos-none foo.c -lresolv' fails identically on all three versions (0.13, 0.14, 0.16). Actual fix: zig cc only searches the -L paths we provide when it can't find a system lib in its bundled SDK stubs. Drop a minimal libresolv.tbd (no exports) into the darwin cargo target dirs that are already on the -L path via CGO_LDFLAGS. Satisfies the link-time -lresolv; at runtime the resolv_* symbols come from libSystem (on real macOS, libresolv.9.dylib is itself a re-exporter of libSystem, so the binary behaves identically to one linked against the real libresolv).
The standalone harness exercised the Rust to CGo to Go chain without going through the CLI. Was useful during bring-up but 'infisical pam rdp access' now covers the same end-to-end path, so the harness is dead code.
Three related fixes from bot review of feat/pam-rdp-mvp: uploader.go: add ResourceTypeRDP to the new-format filename regex tuple. pam-proxy.go calls NewSessionLogger unconditionally before the resource-type switch, so RDP sessions do produce files on disk named pam_session_<id>_windows_expires_<ts>.enc. Without windows in the regex tuple, parsing fell back to the legacy format, which greedy-matched <sessionID>_windows as the session ID. Result: RegisterSession and CleanupPAMSession lookups missed for every RDP session, leaving orphan files until the timestamp-based sweep. pam-proxy.go: gate ResourceTypeRDP in GetSupportedResourceTypes on rdp.IsSupported(). Stub-tier builds were advertising RDP capability to the backend, so the backend would route RDP sessions to a gateway that could only fail them with ErrRdpUnavailable. Now capability advertisement matches runtime capability. rdp-proxy.go: update the writeRDPFile doc comment. Previous text said the .rdp file persists on exit so users can re-open from Finder, but gracefulShutdown explicitly removes it (correctly, since the embedded loopback port is ephemeral). Comment now matches behavior. Also includes the earlier un-committed StartWithReadWriter doc comment expansion explaining the loopback-shim rationale.
- Rename ResourceTypeRDP -> ResourceTypeWindows so the constant name matches the value ("windows") and leaves room for non-RDP Windows session protocols (e.g. WinRM) without overloading the resource type
- Send the full RDP proxy startup banner to stderr so headers and values stay together when stdout is redirected
- Tighten RDP target port validation to reject 0 (unset int zero-value) instead of dialing and failing later
|
💬 Discussion in Slack: #pr-review-cli-191-feat-pam-native-rdp-client-support Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
|
@claude review once |
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, comment @claude review on this pull request to trigger a review.
Consistent naming with bridge_cgo_windows.go.
Simplifies the user experience by only requiring username "infisical" with no password. Removes clipboard copy logic since there's nothing to paste.
The bridge's CredSSP acceptor now expects the user to type the PAM account name (e.g. "test") rather than a fixed "infisical" placeholder, so users see a familiar value when their RDP client prompts. Plumbs accountName from the backend credentials response through the gateway and into the Rust bridge as a new acceptor_username FFI param, distinct from the actual Windows username injected to the target. Falls back to the target username if accountName is empty for older backends.
Intercepts the client's MCS Connect Initial PDU after CredSSP and removes all virtual channels declared in its Client Network Data block before forwarding to the target. Blocks clipboard, drive, printer, audio, smart card, USB, and dynamic-vchan-based features at the gateway level. Mouse, keyboard, and screen ride the implicit MCS I/O channel and are unaffected.
Moves the platform-agnostic HandleConnection method out of the unix/windows cgo files into bridge_cgo_shared.go. Also applies cargo fmt to bridge.rs and ffi.rs to satisfy CI.
…adata Drops the separate acceptor_username plumbing and has the bridge's CredSSP acceptor expect the actual Windows username. The CLI reads pamResponse.Metadata["username"] (set by the backend, mirroring the existing SSH/Postgres pattern) and pre-fills it in the .rdp file so the user doesn't have to type it.
npm-release's needs: includes validate-tag-branch, which is skipped on workflow_dispatch. Without an always() + needs.X.result pattern, the default needs: semantics propagate the skip to npm-release, so a manual non-dry-run release would upload goreleaser artifacts but silently skip npm publish. Mirrors the existing goreleaser job pattern.
The local proxy treated every gateway-side close as a session-level disconnect and shut down the CLI. A failed CredSSP attempt (e.g. wrong username typed in the RDP client) closes the gateway side too, so one typo killed the whole pam rdp access command. Mirrors the kubernetes proxy pattern: per-connection close just ends that connection, the proxy stays up so the user can retry.
|
@claude review once |
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, comment @claude review on this pull request to trigger a review.
Without an explicit release block, the windows goreleaser config defaults to mode: replace and clobbers the linux + darwin artifacts already uploaded to the shared draft (race-y; only manifests when windows finishes after the other two). Mirrors the append config the linux and darwin configs already use.
Description 📣
Adds
infisical pam rdp accessso users can tunnel into PAM-managed Windows targets with their local RDP client. The CLI opens a loopback TCP proxy, the gateway terminates the RDP session with a Rust IronRDP bridge that injects credentials at CredSSP, and all RDP negotiation beyond CredSSP passes through between client and target unchanged.Pairs with the Windows-unblock PR on the infisical repo: Infisical/infisical#6132
Type ✨
Tests 🛠️
Manually validated end-to-end with Microsoft Remote Desktop on macOS against a real Windows Server EC2 target: CLI → relay → gateway → Rust bridge → desktop in the client.